Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get csrf once,avoid init csrfMiddleware from view injection #132

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

TiaNex-Com
Copy link

Q A
Is bugfix? ✔️/❌ no
New feature? ✔️/❌ no
Breaks BC? ✔️/❌ no

there is alsow add a csrftrait to share parmater name bettewn view injection and csrfmiddleware

Returns array of csrf parameters,then merge to common parameters and metaTag parameters.
get csrf token onece ,
@TiaNex-Com TiaNex-Com changed the title get csrf onece,avoid init csrfMiddleware from view injection get csrf once,avoid init csrfMiddleware from view injection Oct 6, 2024
@vjik
Copy link
Member

vjik commented Oct 6, 2024

What problem is being solved here? Is something not working right now?

@TiaNex-Com
Copy link
Author

it initial crsftokeninterface and csrfmiddleware (which initial csrftokeninterface again and other classes not necessary), it's only need to get a post parameter name and Metatag name, the same names used in csrfmiddleware,

@TiaNex-Com
Copy link
Author

TiaNex-Com commented Oct 6, 2024

when the page loading it will fetch csrf token twice from session
sorry, forget to update the CsrfviewInjection file

won't call csrfmiddleware which will inital CsrfTokenInterface again
@vjik
Copy link
Member

vjik commented Oct 6, 2024

when the page loading it will fetch csrf token twice from session sorry, forget to update the CsrfviewInjection file

Only need to refactor CsrfViewInjection::getCsrfParameters() metod to fix it. Why need another changes?

@TiaNex-Com
Copy link
Author

TiaNex-Com commented Oct 7, 2024

when the page loading it will fetch csrf token twice from session sorry, forget to update the CsrfviewInjection file

Only need to refactor CsrfViewInjection::getCsrfParameters() metod to fix it. Why need another changes?

here public function getCsrfParameters(): array
{

    $csrf = new Csrf(
        $this->token->getValue(),
        $this->middleware->getParameterName(),
        $this->middleware->getHeaderName(), 
    );

only get $this->middleware->getParameterName(), and
$this->middleware->getHeaderName(), from csrfmiddleware,

it's not necessary to call csrfmiddleware (call csrftokeninterface again and other useless classes,it's not right to call middleware from view),
so add a Csrftrait to define parmater name and header name,
csrfmiddleware and this CsrfViewInjection all get them from Csrftrait,

CsrfViewInjection implements CommonParametersInjectionInterface, MetaTagsInjectionInterface
this will call twice,
so adding only one CsrfParametersInjectionInterface to avoid ,

from the viewrender get CsrfParameters once to an array,
then merge to common parameters and metatag parameters,

@vjik
Copy link
Member

vjik commented Oct 10, 2024

Anyway you should get parameter/header name from middleware, becuase it can be not default.

@TiaNex-Com
Copy link
Author

TiaNex-Com commented Oct 12, 2024

Anyway you should get parameter/header name from middleware, becuase it can be not default.

yes, if it can be config or customized, it needs a new class which read from configuration, then call the class from middleware or view injection,
if call middleware from view, it called csrftokeninterface again and called other classes not necessary,
it may cause other problems for the middleware is called in sequence,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants